refactor: simplify Hash class with structured parsing helpers#7113
Closed
pierreeurope wants to merge 1 commit intomaplibre:mainfrom
Closed
refactor: simplify Hash class with structured parsing helpers#7113pierreeurope wants to merge 1 commit intomaplibre:mainfrom
pierreeurope wants to merge 1 commit intomaplibre:mainfrom
Conversation
Replace the manual string manipulation in Hash's getHashString, _getCurrentHash, and _removeHash with structured _parseHash and _serializeParams helper methods. The helpers parse the hash fragment into an array of [key, value] pairs and serialize them back, preserving: - Unencoded slashes and colons in values (unlike URLSearchParams) - The distinction between 'key' (no equals) and 'key=' (empty value) - Parameter ordering This makes the code easier to follow and extend, and naturally handles edge cases like trailing ampersands, double ampersands, and leading ampersands through the split-and-filter approach. Closes maplibre#7006
Collaborator
|
Why is this PR better than the other that's already open? |
Contributor
Author
|
You're right, I missed #7073 — apologies for the duplicate. That PR takes the correct approach of using actual |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactor the
Hashclass to replace manual string building/parsing with structured_parseHash()and_serializeParams()helper methods.Closes #7006
Motivation
As noted in #7006, the Hash class does a lot of manual string building and parsing, making it tricky to extend or maintain. This PR extracts the parsing and serialization into dedicated methods.
Approach
Rather than using
URLSearchParamsdirectly (which encodes slashes as%2Fand colons as%3A, breaking the hash value format), this PR introduces two lightweight helpers:_parseHash(): Parses the hash fragment into anArray<[string, string | null]>— preserving parameter order, the distinction betweenkey(no equals) andkey=(empty value), and unencoded characters in values_serializeParams(): Serializes the array back to a&-separated stringThese naturally handle edge cases (trailing/double/leading ampersands) through the split-and-filter approach.
Changes
getHashString(): Uses_parseHash()+ find/update/push +_serializeParams()instead of manual.split('&').map().filter()_getCurrentHash(): Uses_parseHash()+.find()instead of manual.split('&').map().forEach()_removeHash(): Uses_parseHash()+.filter()+_serializeParams()instead of manual string replacementTests
All 52 existing tests pass unchanged — no behavior changes.